ENG-696: structured publish result contract - #286

Adversarial review (deep) — head 3c80ef3e, base origin/staging 936f7083

Reviewed together with anton#319. Deep fan-out: 7 finders across the flagged dimensions, every finding put to 1–3 agents prompted to refute it. 20 findings raised, 8 survived.

The behaviour is sound. The risk I went in expecting — a JSON blob reaching something that string-matched prose — did not materialise: there is no prose-parsing consumer in the renderer, cowork_evals or mdb-ai, and the ImportError shim makes old-anton behaviour byte-identical. What is wrong here is verification, not behaviour.

1. HIGH — the envelope path is unreachable in CI and unpinned by any assertion, so the whole migration reverts green

confirmed · mutation-verified twice by independent reviewers · introduced by this PR

Two halves of one mechanism.

Half A — CI cannot execute the branch. uv.lock:147:

source = { git = "https://github.com/mindsdb/anton.git?branch=main#fdaa2f1d993c062a60daf846b9d1441799977826" }

side_effect.py is absent from anton's origin/main, origin/staging and that locked sha (git cat-file -e → does not exist for all three). tests-unit.yml:30 runs uv sync --group dev against that lock, so tools.py:209 always raises ImportError and takes SideEffectResult = None.

Mutation: inserting raise RuntimeError("MUTANT") after both fallback guards (tools.py:214 and :221) and running the full suite gives 1089 passed, 4 skipped — identical to baseline. Zero tests reach the envelope.

Half B — even when it does run, nothing pins it. With anton#319's side_effect.py force-injected so the branch is live, all 20 publish tests still pass, because every assertion accepts either shape:

tests/test_stable_publish_url.py:256   assert "https://4nton.ai/a/uuid-1" in getattr(out, "content", out)
tests/test_harness_publish_access.py:38 assert "Published" in getattr(out, "content", out)

to_outcome() embeds message inside the JSON, so the substring holds either way. Four mutations against a pristine copy with the envelope live — all survived at 20 passed:

• revert the migration (if SideEffectResult is None → if True, both sites)
• drop resource_id / idempotency_key / content_hash
• drop committed_at=now_iso()
• invert both verdicts — every failed publish reports ok=True

That last one is why this is high rather than a test nit. anton/core/session.py:883 does if ok is not None: is_error = not ok, and its own docstring names the consequence: "a genuine failure … RESETS it, which is how the ENG-836 driver ping-pong … kept the breaker asleep at ~4.95M tokens." An inverted ok on publish silently disarms the ENG-1276 circuit breaker — the mechanism ENG-696 exists to arm.

The new test_tool_publish_falls_back_to_string_on_old_anton is a good test (deleting the try/except does fail it) but it covers the branch CI already takes by default, not the deliverable.

And it activates without a gate. The shipped wheel carries no git pin — Requires-Dist: anton-agent<3,>=2.26.8.9.1 — and publish-staging.yml:101-108 rewrites the dep to the latest PyPI rc at publish time. So the untested branch goes live on anton's next release: no lock bump, no PR, no review.

Smallest fix — ~20 lines, needs no anton merge. Invert the patch.dict the PR already uses: patch the module to a stub rather than to None.

def test_tool_publish_returns_envelope_on_new_anton(tmp_path):
    stub = types.ModuleType("anton.core.tools.side_effect")
    stub.SideEffectResult = SideEffectResult      # or a minimal local dataclass
    stub.now_iso = lambda: "2026-08-11T00:00:00Z"
    with patch.dict(sys.modules, {"anton.core.tools.side_effect": stub}), \
         patch.object(tools_mod, "_publish_artifact",
             lambda p, access=None: {"url": "https://4nton.ai/a/uuid-1",
                                     "result": {"report_id": "rep-1", "md5": "deadbeef"}}):
        out = _run(tools_mod._cowork_publish_or_preview(_FakeSession(tmp_path), {...}))
    payload = json.loads(out.content)
    assert out.ok is True
    assert payload["resource_id"] == "rep-1" and payload["idempotency_key"] == "rep-1"
    assert payload["external_url"] == "https://4nton.ai/a/uuid-1"
    assert payload["content_hash"] == "md5:deadbeef" and payload["committed_at"]

Plus a failure twin asserting out.ok is False and out.reason == "missing_api_key". Note _publish_artifact has to be stubbed to return the nested {"result": {"report_id", "md5"}} shape — no current test ever makes those non-empty, so the field mapping is unexercised even in principle.

Fails before / passes after: the pair fails against all four mutations above (the revert mutant returns a str, so out.content raises AttributeError; the inverted mutant fails out.ok is True) and passes on HEAD with the stub — independent of which anton is installed.

2. MEDIUM — the envelope activates on four channels at four different times, and hosted never activates on merge

confirmed · pre-existing topology, first consequential here · no code fix — PR-body + QA item

• Hosted/Docker: frozen. Dockerfile:26,31 uv sync --frozen → the locked fdaa2f1, which has no side_effect.py. Nothing forces a bump: rewriting the pin to an older commit still passes uv lock --check in 12ms, because pyproject.toml:79 only requires branch = "main".
• Desktop staging: first. cowork/src/main/server-source.ts:196-215 lets cowork-server's own pin decide, and build-macos-pkg.yml:252-256 passes ANTON_REF, so a staging build resolves anton@staging and picks up #319 the moment it merges — ahead of main.
• Desktop main / PyPI: on anton's next release.

No functional breakage — the fallback keeps publishing byte-identical. But "merge #319, then verify the envelope" will be verified against a build still emitting prose and read as a failed fix. That is exactly the team's QA Step 0 shape.

Suggested: QA Step 0 should say "confirm which anton the build carries before judging output shape" — the signal already exists and is free: cowork/api/v1/endpoints/health.py:35 returns anton_version, and build_info.py:147-151 stamps it on every Langfuse trace. Add to expected side effects that are NOT regressions: a prose-string publish means an old pin, not a failed fix.

Checked and produced nothing

• Prose consumers. Inventoried exhaustively across cowork, cowork_evals and mdb-ai and structurally immune: anton emits tool content only for scratchpad (session.py:3552, :3699, both name-guarded); the generic branch yields StreamTaskProgress with no content. Artifact cards come from a directory diff (harness.py:361), the "Shared" pill from the REST sidecar, persisted tool rows are hidden (conversations.py:575). Grep for every removed prose string (PUBLISH FAILED, Published successfully, no view URL, No Minds API key, Artifact store unavailable) → zero hits. An executed probe drove the real _cowork_publish_or_preview through dispatch_tool into format_responses_stream: envelope in SSE: False. Worth one line in the PR body so a reviewer doesn't re-litigate it.
• Desktop timeline turning failed publishes red. Refuted: stream_formatter.py:321 gates thought.tool_call.end on event.id in progress_tool_ids, populated only by tool_progress, which no production handler emits. ok never reaches the renderer for these tools.
• publish's first-publish idempotency window. Real pre-existing gap (server-mints report_id, persisted only after a successful round-trip; publish.py:350-353 swallows the write failure) but wholly outside this diff, and the envelope emits explicit null there rather than a false claim. Separate ticket.
• -> str removed rather than widened on _cowork_publish_or_preview. Style only — registry.py:104-114 documents str | ToolOutcome as the handler contract and normalises both; no type-checker runs in CI.

Recommendation: the top finding is worth changes requested — one stub test closes it and costs ~20 lines. Leaving this as a COMMENT so the formal state stays a human call.

Merge order: this PR's body already says it should land after anton#319 — worth noting that means after #319 reaches anton main, not staging, since that is what the lock resolves.

Requesting changes on one item — the behaviour is right, the verification isn't there.

The envelope path is never executed, and nothing pins it. Detail and evidence in my review above; the short version:

• uv.lock:147 pins anton at fdaa2f1, which has no side_effect.py — so CI always takes the ImportError fallback. Inserting raise RuntimeError("MUTANT") after both guards (tools.py:214, :221) still gives 1089 passed, 4 skipped — identical to baseline.
• Force-injecting the module so the branch is live, all 20 publish tests still pass under four separate sabotage mutations: reverting the migration; dropping resource_id/idempotency_key/content_hash; dropping committed_at; and inverting both ok verdicts. Every assertion is getattr(out, "content", out) plus a substring, and to_outcome() embeds message inside the JSON, so the substring holds either way.

The inverted-verdict case is why this is blocking rather than a nit: anton/core/session.py:883 does is_error = not ok, so a publish failure reporting ok=True silently disarms the ENG-1276 circuit breaker — the mechanism ENG-696 exists to arm. And it activates without a gate: the shipped wheel carries no git pin, and publish-staging.yml:101-108 rewrites the dep to the latest PyPI rc, so the untested branch goes live on anton's next release.

~20 lines closes it, and it needs no anton merge — invert the patch.dict this PR already uses: patch the module to a stub rather than to None, assert out.ok and the five envelope fields on the success path, plus a failure twin asserting out.ok is False and out.reason == "missing_api_key". Full snippet in the review above. Note _publish_artifact has to be stubbed to return the nested {"result": {"report_id", "md5"}} shape — no current test makes those non-empty, so the field mapping is unexercised even in principle.

Nothing else on the PR is blocking. The prose→JSON switch is safe (no consumer anywhere string-matched the old format — verified by executing the real dispatch path and grepping every removed prose string across cowork, cowork_evals and mdb-ai), and the ImportError shim makes old-anton behaviour byte-identical.

Re-checked at dac6b2b6. The blocking finding is closed — clearing my earlier CHANGES_REQUESTED.

f7d76296 does exactly what was missing, and I re-ran every probe from the original review rather than reading the commit message, because a mutation table is only worth what someone else can reproduce.

Each mutation was confirmed applied before its result was trusted — a no-op edit and a surviving mutation look identical.

The inverted-verdict row is the one that mattered: anton/core/session.py:883 does is_error = not ok, so a publish failure reporting ok=True would have silently disarmed the ENG-1276 circuit breaker. That is now pinned from both directions.

Why this approach is the right one. Injecting a stub anton.core.tools.side_effect via patch.dict(sys.modules, ...) makes the tests independent of which anton is installed — uv.lock:147 still pins fdaa2f1, which has no side_effect.py, so a test that waited on the real module would still be dormant in CI today. It also populates the nested {"result": {report_id, md5}} shape that no previous test made non-empty, so the field mapping is genuinely exercised rather than defaulted through.

The failure twin is a nice addition beyond what I asked for: asserting committed_at is None and resource_id is None on the failure path pins the absence of a spurious commit, not just the verdict.

Two notes, neither blocking:

• The commit message lists four mutations including "reverting the migration." This PR contains no migration — three files at both shas (tools.py plus the two test files) — so that one isn't reproducible from this diff. The three that apply to the deliverable all bite, so it doesn't change anything.
• Full suite here: 1098 passed, 4 skipped, 1 failed — tests/test_comments_layer.py::test_serve_injects_only_with_flag. Pre-existing, not from this branch: it fails identically on clean origin/staging (b31f83d). Flagging so it isn't mistaken for fallout.

Everything from the first review still holds: the prose to JSON switch is safe (no consumer anywhere string-matched the old format — verified by executing the real dispatch path and grepping every removed prose string across cowork, cowork_evals and mdb-ai), and the ImportError shim keeps old-anton behaviour byte-identical.